From b7f49181c916f27256935b8bef244cfbb590d913 Mon Sep 17 00:00:00 2001 From: "cl349@firebug.cl.cam.ac.uk" Date: Tue, 2 Aug 2005 18:04:28 +0000 Subject: [PATCH] Implement watching of nodes which don't exist. Requires permission check every time event is generated. Requires generalization of permissions: ask arbitrary number of parents whether it's OK to tell about node (eg. watching /dir/subdir/x when /dir is deleted: root permissions will now determine whether we fire event). Add test that we don't leak information on whether a file exists or not. Signed-off-by: Rusty Russel Signed-off-by: Christian Limpach --- tools/xenstore/testsuite/07watch.sh | 19 ++++++++ tools/xenstore/xenstored_core.c | 70 ++++++++++++++++------------- tools/xenstore/xenstored_watch.c | 17 +++++-- tools/xenstore/xs_lib.h | 2 +- 4 files changed, 72 insertions(+), 36 deletions(-) diff --git a/tools/xenstore/testsuite/07watch.sh b/tools/xenstore/testsuite/07watch.sh index 00af679a29..f3cb3e32d4 100644 --- a/tools/xenstore/testsuite/07watch.sh +++ b/tools/xenstore/testsuite/07watch.sh @@ -160,3 +160,22 @@ async 2 write /test3 create contents 1 waitwatch' | ./xs_test 2>&1`" = "1:/test2/foo:token 1:contents2 1:waitwatch timeout" ] + +# We can watch something which doesn't exist. +[ "`echo '1 watch /dir/subdir token +2 mkdir /dir/subdir +1 waitwatch' | ./xs_test 2>&1`" = "1:/dir/subdir:token" ] + +# If we don't have permission, we won't see event (rm). +[ "`echo '1 setid 1 +1 watch /dir/subdir token +setperm /dir 0 NONE +rm /dir/subdir +1 waitwatch' | ./xs_test 2>&1`" = "1:waitwatch timeout" ] + +# If we don't have permission, we won't see event (create). +[ "`echo '1 setid 1 +1 watch /dir/subdir token +mkdir /dir/subdir +write /dir/subdir/entry create contents +1 waitwatch' | ./xs_test 2>&1`" = "1:waitwatch timeout" ] diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c index ad35ed2f89..c836acfac0 100644 --- a/tools/xenstore/xenstored_core.c +++ b/tools/xenstore/xenstored_core.c @@ -707,7 +707,7 @@ static enum xs_perm_type perm_for_id(domid_t id, /* Owners and tools get it all... */ if (!id || perms[0].id == id) - return XS_PERM_READ|XS_PERM_WRITE|XS_PERM_CREATE|XS_PERM_OWNER; + return XS_PERM_READ|XS_PERM_WRITE|XS_PERM_OWNER; for (i = 1; i < num; i++) if (perms[i].id == id) @@ -716,20 +716,13 @@ static enum xs_perm_type perm_for_id(domid_t id, return perms[0].perms; } -/* We have a weird permissions system. You can allow someone into a - * specific node without allowing it in the parents. If it's going to - * fail, however, we don't want the errno to indicate any information - * about the node. */ -static int check_with_parents(struct connection *conn, const char *node, - int errnum) +/* What do parents say? */ +static enum xs_perm_type ask_parents(struct connection *conn, + const char *node) { struct xs_permissions *perms; unsigned int num; - /* We always tell them about memory failures. */ - if (errnum == ENOMEM) - return errnum; - do { node = get_parent(node); perms = get_perms(conn->transaction, node, &num); @@ -741,10 +734,23 @@ static int check_with_parents(struct connection *conn, const char *node, if (!perms) corrupt(conn, "No permissions file at root"); - if (!(perm_for_id(conn->id, perms, num) & XS_PERM_READ)) - return EACCES; + return perm_for_id(conn->id, perms, num); +} - return errnum; +/* We have a weird permissions system. You can allow someone into a + * specific node without allowing it in the parents. If it's going to + * fail, however, we don't want the errno to indicate any information + * about the node. */ +static int errno_from_parents(struct connection *conn, const char *node, + int errnum) +{ + /* We always tell them about memory failures. */ + if (errnum == ENOMEM) + return errnum; + + if (ask_parents(conn, node) & XS_PERM_READ) + return errnum; + return EACCES; } char *canonicalize(struct connection *conn, const char *node) @@ -776,24 +782,26 @@ bool check_node_perms(struct connection *conn, const char *node, } perms = get_perms(conn->transaction, node, &num); - /* No permissions. If we want to create it and - * it doesn't exist, check parent directory. */ - if (!perms && errno == ENOENT && (perm & XS_PERM_CREATE)) { - char *parent = get_parent(node); - if (!parent) - return false; - perms = get_perms(conn->transaction, parent, &num); - } - if (!perms) { - errno = check_with_parents(conn, node, errno); + if (perms) { + if (perm_for_id(conn->id, perms, num) & perm) + return true; + errno = EACCES; return false; } - if (perm_for_id(conn->id, perms, num) & perm) - return true; + /* If it's OK not to exist, we consult parents. */ + if (errno == ENOENT && (perm & XS_PERM_ENOENT_OK)) { + if (ask_parents(conn, node) & perm) + return true; + /* Parents say they should not know. */ + errno = EACCES; + return false; + } - errno = check_with_parents(conn, node, EACCES); + /* They might not have permission to even *see* this node, in + * which case we return EACCES even if it's ENOENT or EIO. */ + errno = errno_from_parents(conn, node, errno); return false; } @@ -930,9 +938,9 @@ static void do_write(struct connection *conn, struct buffered_data *in) if (streq(vec[1], XS_WRITE_NONE)) mode = XS_PERM_WRITE; else if (streq(vec[1], XS_WRITE_CREATE)) - mode = XS_PERM_WRITE|XS_PERM_CREATE; + mode = XS_PERM_WRITE|XS_PERM_ENOENT_OK; else if (streq(vec[1], XS_WRITE_CREATE_EXCL)) - mode = XS_PERM_WRITE|XS_PERM_CREATE; + mode = XS_PERM_WRITE|XS_PERM_ENOENT_OK; else { send_error(conn, EINVAL); return; @@ -951,7 +959,7 @@ static void do_write(struct connection *conn, struct buffered_data *in) } /* Not going to create it? */ - if (!(mode & XS_PERM_CREATE)) { + if (streq(vec[1], XS_WRITE_NONE)) { send_error(conn, ENOENT); return; } @@ -985,7 +993,7 @@ static void do_write(struct connection *conn, struct buffered_data *in) static void do_mkdir(struct connection *conn, const char *node) { node = canonicalize(conn, node); - if (!check_node_perms(conn, node, XS_PERM_WRITE|XS_PERM_CREATE)) { + if (!check_node_perms(conn, node, XS_PERM_WRITE|XS_PERM_ENOENT_OK)) { send_error(conn, errno); return; } diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c index 1c42b72ced..a2727e46e4 100644 --- a/tools/xenstore/xenstored_watch.c +++ b/tools/xenstore/xenstored_watch.c @@ -95,10 +95,19 @@ static int destroy_watch_event(void *_event) return 0; } -static void add_event(struct watch *watch, const char *node) +static void add_event(struct connection *conn, + struct watch *watch, const char *node) { struct watch_event *event; + /* Check read permission: no permission, no watch event. + * If it doesn't exist, we need permission to read parent. + */ + if (!check_node_perms(conn, node, XS_PERM_READ|XS_PERM_ENOENT_OK)) { + fprintf(stderr, "No permission for %s\n", node); + return; + } + if (watch->relative_path) { node += strlen(watch->relative_path); if (*node == '/') /* Could be "" */ @@ -132,9 +141,9 @@ void fire_watches(struct connection *conn, const char *node, bool recurse) list_for_each_entry(watch, &i->watches, list) { if (is_child(node, watch->node)) - add_event(watch, node); + add_event(i, watch, node); else if (recurse && is_child(watch->node, node)) - add_event(watch, watch->node); + add_event(i, watch, watch->node); else continue; /* If connection not doing anything, queue this. */ @@ -206,7 +215,7 @@ void do_watch(struct connection *conn, struct buffered_data *in) relative = !strstarts(vec[0], "/"); vec[0] = canonicalize(conn, vec[0]); - if (!check_node_perms(conn, vec[0], XS_PERM_READ)) { + if (!is_valid_nodename(vec[0])) { send_error(conn, errno); return; } diff --git a/tools/xenstore/xs_lib.h b/tools/xenstore/xs_lib.h index 97b72c8c7e..efdee0bad6 100644 --- a/tools/xenstore/xs_lib.h +++ b/tools/xenstore/xs_lib.h @@ -30,7 +30,7 @@ enum xs_perm_type { XS_PERM_READ = 1, XS_PERM_WRITE = 2, /* Internal use. */ - XS_PERM_CREATE = 4, + XS_PERM_ENOENT_OK = 4, XS_PERM_OWNER = 8, }; -- 2.30.2